Various refactors to the LTO handling code #143388
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Most uses of it either contain a fat or thin lto module. Only WorkItem::LTO could contain both, but splitting that enum variant doesn't complicate things much.
As opposed to sending a message to the coordinator thread.
This will allow merging all fat LTO steps together. In addition it reduces the amount of work done on the coordinator thread without jobserver token.
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
A-LLVM
|
Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in compiler/rustc_codegen_llvm/src/builder/autodiff.rs cc @ZuseZ4 |
bjorn3
commented
Jul 4, 2025
| autodiff: autodiff_items.clone(), | ||
| }, | ||
| 0, | ||
| )); | ||
| if cgcx.parallel { | ||
| helper.request_token(); | ||
| } |
There was a problem hiding this comment.
I'm not sure if this is necessary for fat LTO given that only a single thread will be doing LTO work and we have the implicit token anyway. It is pre-existing though and the token should be immediately yielded back to the jobserver if we indeed don't actually use it.
There was a problem hiding this comment.
My latest cleanup pr removes the clone, but you can land this pr first, as you say it didn't really hurt.
There was a problem hiding this comment.
I was actually talking about the helper.request_token() call.
|
@bors r+ |
bors
added
S-waiting-on-bors
fmease
added a commit
to fmease/rust
that referenced
this pull request
Jul 16, 2025
…rrors Various refactors to the LTO handling code In particular reducing the sharing of code paths between fat and thin-LTO and making the fat LTO implementation more self-contained. This also moves some autodiff handling out of cg_ssa into cg_llvm given that Enzyme only works with LLVM anyway and an implementation for another backend may do things entirely differently. This will also make it a bit easier to split LTO handling out of the coordinator thread main loop into a separate loop, which should reduce the complexity of the coordinator thread.
bors
added a commit
that referenced
this pull request
Jul 17, 2025
Rollup of 15 pull requests Successful merges: - #142304 (tests: Add `RUST_BACKTRACE` and `-Cpanic` revisions to `panic-main.rs` test) - #143388 (Various refactors to the LTO handling code) - #143409 (Enable xgot feature for mips64 musl targets) - #143592 (UWP: link ntdll functions using raw-dylib) - #143595 (add `const_make_global`; err for `const_allocate` ptrs if didn't call) - #143678 (Added error for invalid char cast) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143829 (Trim `BorrowedCursor` API) - #143846 (pass --gc-sections if -Zexport-executable-symbols is enabled and improve tests) - #143851 (ci cleanup: rustdoc-gui-test now installs browser-ui-test) - #143856 (Linting public reexport of private dependencies) - #143895 (Dont collect assoc ty item bounds from trait where clause for host effect predicates) - #143922 (Improve path segment joining) - #143964 (Fix handling of SCRIPT_ARG in docker images) - #144016 (trait_sel: `MetaSized` always holds temporarily) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
that referenced
this pull request
Jul 17, 2025
Rollup of 15 pull requests Successful merges: - #142304 (tests: Add `RUST_BACKTRACE` and `-Cpanic` revisions to `panic-main.rs` test) - #143388 (Various refactors to the LTO handling code) - #143409 (Enable xgot feature for mips64 musl targets) - #143592 (UWP: link ntdll functions using raw-dylib) - #143595 (add `const_make_global`; err for `const_allocate` ptrs if didn't call) - #143678 (Added error for invalid char cast) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143829 (Trim `BorrowedCursor` API) - #143851 (ci cleanup: rustdoc-gui-test now installs browser-ui-test) - #143856 (Linting public reexport of private dependencies) - #143895 (Dont collect assoc ty item bounds from trait where clause for host effect predicates) - #143922 (Improve path segment joining) - #143964 (Fix handling of SCRIPT_ARG in docker images) - #144002 (Update poison.rs) - #144016 (trait_sel: `MetaSized` always holds temporarily) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Jul 17, 2025
Rollup merge of #143388 - bjorn3:lto_refactors, r=compiler-errors Various refactors to the LTO handling code In particular reducing the sharing of code paths between fat and thin-LTO and making the fat LTO implementation more self-contained. This also moves some autodiff handling out of cg_ssa into cg_llvm given that Enzyme only works with LLVM anyway and an implementation for another backend may do things entirely differently. This will also make it a bit easier to split LTO handling out of the coordinator thread main loop into a separate loop, which should reduce the complexity of the coordinator thread.
rust-bors bot
added a commit
that referenced
this pull request
Jul 17, 2025
Various refactors to the LTO handling code (part 2) Continuing from #143388 this removes a bit of dead code and moves the LTO symbol export calculation from individual backends to cg_ssa.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In particular reducing the sharing of code paths between fat and thin-LTO and making the fat LTO implementation more self-contained. This also moves some autodiff handling out of cg_ssa into cg_llvm given that Enzyme only works with LLVM anyway and an implementation for another backend may do things entirely differently. This will also make it a bit easier to split LTO handling out of the coordinator thread main loop into a separate loop, which should reduce the complexity of the coordinator thread.